Skip to content

Reject channels if the total reserves are larger than the funding #1454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 28, 2022

Conversation

TheBlueMatt
Copy link
Collaborator

The full_stack_target fuzzer managed to find a subtraction
underflow in the new Channel::get_htlc_maximum function where we
subtract both sides' reserve values from the channel funding. Such
a channel is obviously completely useless, so we should reject it
during opening instead of integer-underflowing later.

Thanks to Chaincode Labs for providing the fuzzing resources which
found this bug!

@TheBlueMatt TheBlueMatt added this to the 0.0.107 milestone Apr 26, 2022
@TheBlueMatt TheBlueMatt force-pushed the 2022-04-fuzz-underflow branch from 70957cd to 3bc962a Compare April 26, 2022 21:07
jkczyz
jkczyz previously approved these changes Apr 28, 2022
The `full_stack_target` fuzzer managed to find a subtraction
underflow in the new `Channel::get_htlc_maximum` function where we
subtract both sides' reserve values from the channel funding. Such
a channel is obviously completely useless, so we should reject it
during opening instead of integer-underflowing later.

Thanks to Chaincode Labs for providing the fuzzing resources which
found this bug!
The calculation uses the reserve, so we should mention it in the
error we send to our peers.
@TheBlueMatt
Copy link
Collaborator Author

Oops, missed a test had the same error in it, fixed that and made the fixup a Real Commit instead.

@TheBlueMatt TheBlueMatt merged commit dc8479a into lightningdevkit:main Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants